Skip to content

Conversation

@ibihim
Copy link
Collaborator

@ibihim ibihim commented Mar 14, 2025

What

Add unit tests to test the OIDC logic.

Why

There was an issue before and a comment isn't good enough to protect someone in the future from breaking it 😄

@ibihim ibihim force-pushed the ibihim/2025-03-14_nil-type-non-nil-interface-issue branch from ac5970b to 61966f1 Compare March 14, 2025 11:40
return ts
}

func TestNewOIDCAuthenticator(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only tests the CAs used in the authenticator.

Either rename the test or dedicate some time to turn this into table tests for the whole authenticator config (the latter is preferred).

defer ts.Close()

// Override the default transport to skip TLS verification.
// This is needed because ts uses a self-signed certificate.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not a reason to do this. Why do you actually need it? Who uses the client?


// Override the default transport to skip TLS verification.
// This is needed because ts uses a self-signed certificate.
// This should be legit for testing purpose, even with parallel test cases run at the same time.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is legit.

@ibihim
Copy link
Collaborator Author

ibihim commented Sep 19, 2025

not worth the effort

@ibihim ibihim closed this Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants